fix: stop choppy screen resize when switching modes#1187
Conversation
Mode switches animated phone composer reserve because searchMode updated before the pathname landed, briefly leaving isStandaloneModeHome false and running the 200ms padding transition. Detect mode homes from pathname only, navigate without optimistic mode state, and limit padding transitions to scroll-hide. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
After the reserve-flip fix, related choppiness remained from eager crossModeSearch chrome updates, inherited phone scroll/hide across mode homes, a hero-portal null gap while slots rebound, a taller mode-home loading skeleton, and services/forms contentAlign jumping after registry load. Navigate out of the dashboard without rewriting chrome, reset scroll-hide on pathname change, keep the default composer until the portal attaches, align the skeleton to the shell header token, and keep loading homes top-aligned on phone. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR centralizes standalone mode-home detection, changes navigation to follow URL ownership, standardizes route loading skeletons, adjusts mobile layout transitions, prevents unnecessary remounts, stabilizes composer handoffs, dynamically loads ChangesMode routing and shell stabilization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reset bottom-composer hidden state during render on pathname change instead of setState-in-effect, and drop the unused hero-portal fallback flag now that the default composer stays mounted until the host attaches. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #5251 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Append review of mode/page loading and navigation architecture at HEAD 0ef62ff: P1 shell bundle + hydration blanking; residual remount/tools dual. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Parallel loading review found hard-load blanking from ClientHydrationBoundary, mismatched/missing mode-home loading skeletons, forms catalog in the client chunk, ClinicalDashboard static weight on namespaced routes, sidebar column animation on remount, forms query remounts, and document viewer remounts on page flips. Paint RSC children immediately, align ModeHomeRouteLoading, wire mode-home loading.tsx files, server-pass the default form slug, dynamic-import ClinicalDashboard, gate sidebar transitions after mount, and stop unnecessary remount keys. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Keeps ClinicalDashboard inside the maintainability budget after the loading-performance pass, and shares the remount-safe sidebar transition gate with GlobalSearchShell. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Parallel loading review follow-up (this tip)Ran explore×3 + live debug across modes/pages. Confirmed: Answer↔namespaced remount dominates settle (~0.6–1.2s); hero portal rebinds; registry pop-in after paint. Landed quick wins: remove ClientHydrationBoundary blanking; ModeHomeRouteLoading phone top-align; mode-home Residual: unify dashboard/standalone chrome; stable shell-owned hero slot; registry abort+LRU/summary; Tools dual entry
|
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tests/document-detail-performance.test.ts (1)
21-24: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd behavioral coverage for same-document transitions.
These assertions verify source text only. Add a runtime test that changes page/chunk for the same document and confirms the viewer updates without remounting or reloading, while a different document still remounts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/document-detail-performance.test.ts` around lines 21 - 24, Add runtime coverage in the document detail performance test around the DocumentViewer flow: navigate between pages or chunks for the same document and assert the existing viewer instance remains mounted without reloading, then switch to a different document and assert the viewer remounts. Keep the existing source-text assertions and use the test’s established navigation and viewer-loading helpers.src/components/forms/forms-home-page.tsx (1)
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the form-detail href.
Use the existing route ownership/catalog mechanism for this internal destination rather than adding
"/forms/..."inline here.As per coding guidelines, “build hrefs from the existing application source catalogs rather than scattered hardcoded strings.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/forms/forms-home-page.tsx` at line 43, Update the href in the forms home page component to use the existing route ownership/catalog mechanism for the form-detail destination, passing defaultFormSlug through that centralized route builder instead of constructing the "/forms/..." path inline.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(search-app)/page.tsx:
- Around line 81-87: Update the redirect built by appModeHomeHref in the
initialSearchMode services/forms branch to preserve the supported query alias by
reading and forwarding the query parameter alongside q. Ensure requests using
query, such as query=mania with run=1, retain the search text and
submitted-search state when redirected.
In `@src/components/clinical-dashboard/master-search-header.tsx`:
- Around line 1890-1895: Update renderSearchComposer and its desktop fallback
condition so the hero-specific sm:hidden class is applied only when the desktop
home composer portal is actually attached, not merely when isHeroDesktopComposer
is true. Ensure the default composer remains visible while
desktopHomeComposerActive is true and desktopHomeComposerHost is null, then
validate with npm run ensure and npm run verify:ui.
In `@src/lib/private-search-scope.ts`:
- Around line 75-80: Update removePrivateScopeRefFromUrl to include hash in its
Location shape and preserve the current fragment when constructing the
replacement URL. Append location.hash after the pathname and updated query
string so removing scopeRef does not change the active in-page section.
---
Nitpick comments:
In `@src/components/forms/forms-home-page.tsx`:
- Line 43: Update the href in the forms home page component to use the existing
route ownership/catalog mechanism for the form-detail destination, passing
defaultFormSlug through that centralized route builder instead of constructing
the "/forms/..." path inline.
In `@tests/document-detail-performance.test.ts`:
- Around line 21-24: Add runtime coverage in the document detail performance
test around the DocumentViewer flow: navigate between pages or chunks for the
same document and assert the existing viewer instance remains mounted without
reloading, then switch to a different document and assert the viewer remounts.
Keep the existing source-text assertions and use the test’s established
navigation and viewer-loading helpers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f6ed9aa-184b-4eeb-89ab-eb5be5743760
📒 Files selected for processing (34)
docs/branch-review-ledger.mddocs/search-chrome-behaviour.mdscripts/check-maintainability-budgets.mjssrc/app/(search-app)/differentials/loading.tsxsrc/app/(search-app)/documents/[id]/page.tsxsrc/app/(search-app)/dsm/loading.tsxsrc/app/(search-app)/factsheets/loading.tsxsrc/app/(search-app)/favourites/loading.tsxsrc/app/(search-app)/forms/loading.tsxsrc/app/(search-app)/forms/page.tsxsrc/app/(search-app)/formulation/loading.tsxsrc/app/(search-app)/page.tsxsrc/app/(search-app)/specifiers/loading.tsxsrc/app/(search-app)/therapy-compass/loading.tsxsrc/app/globals.csssrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/global-search-shell.tsxsrc/components/clinical-dashboard/master-search-header.tsxsrc/components/clinical-dashboard/shared-search-app-shell.tsxsrc/components/clinical-dashboard/use-hide-on-scroll.tssrc/components/clinical-dashboard/use-sidebar-column-transition.tssrc/components/forms/forms-home-page.tsxsrc/components/forms/forms-search-results-page.tsxsrc/components/mode-home-page-skeleton.tsxsrc/components/services/services-home-page.tsxsrc/lib/private-search-scope.tssrc/lib/search-route-ownership.tstests/document-detail-performance.test.tstests/forms-client-boundary.test.tstests/mode-home-loading-contract.test.tstests/mode-home-main-align.test.tstests/search-route-ownership.test.tstests/therapy-compass-mode-wiring.test.tstests/ui-phone-scroll.spec.ts
| if (initialSearchMode === "services" || initialSearchMode === "forms") { | ||
| redirect( | ||
| appModeHomeHref(initialSearchMode, { | ||
| query: firstSearchParam(params.q)?.trim(), | ||
| focus: firstSearchParam(params.focus) === "1", | ||
| run: firstSearchParam(params.run) === "1", | ||
| }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the supported query alias.
GlobalStandaloneSearchShellClient reads q ?? query, but this redirect forwards only q. /?mode=services&query=mania&run=1 therefore lands without a query or submitted search.
Proposed fix
- query: firstSearchParam(params.q)?.trim(),
+ query: firstSearchParam(params.q ?? params.query)?.trim(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (initialSearchMode === "services" || initialSearchMode === "forms") { | |
| redirect( | |
| appModeHomeHref(initialSearchMode, { | |
| query: firstSearchParam(params.q)?.trim(), | |
| focus: firstSearchParam(params.focus) === "1", | |
| run: firstSearchParam(params.run) === "1", | |
| }), | |
| if (initialSearchMode === "services" || initialSearchMode === "forms") { | |
| redirect( | |
| appModeHomeHref(initialSearchMode, { | |
| query: firstSearchParam(params.q ?? params.query)?.trim(), | |
| focus: firstSearchParam(params.focus) === "1", | |
| run: firstSearchParam(params.run) === "1", | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(search-app)/page.tsx around lines 81 - 87, Update the redirect
built by appModeHomeHref in the initialSearchMode services/forms branch to
preserve the supported query alias by reading and forwarding the query parameter
alongside q. Ensure requests using query, such as query=mania with run=1, retain
the search text and submitted-search state when redirected.
| {/* Keep the default composer visible until the hero portal is actually | ||
| attached. Hiding on slotId alone left a null gap while the mode-home | ||
| slot remounted / MutationObserver rebound (mode-switch flicker). */} | ||
| {desktopHomeComposerActive && desktopHomeComposerHost ? null : renderSearchComposer("default")} | ||
| {desktopHomeComposerActive && desktopHomeComposerHost | ||
| ? createPortal(renderSearchComposer("desktop-home"), desktopHomeComposerHost) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/components/clinical-dashboard/master-search-header.tsx"
echo "== file exists and size =="
wc -l "$file"
echo "== outline around relevant symbols =="
ast-grep outline "$file" --match renderSearchComposer --view expanded || true
ast-grep outline "$file" --match MasterSearchHeader --view expanded || true
echo "== relevant renderer slice =="
sed -n '1340,1425p' "$file"
echo "== relevant fallback/portals slice =="
sed -n '1860,1910p' "$file"
echo "== state references for desktopHomeComposerActive and desktopHomeComposerHost =="
rg -n "desktopHomeComposer(Active|Host)|desktopSearchPlacement|isHeroDesktopComposer|renderSearchComposer\\(\"default\"\\)" "$file"Repository: BigSimmo/Database
Length of output: 8857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/components/clinical-dashboard/master-search-header.tsx"
# Read-only behavioral probe: parse the render expression conditions without executing repo code.
python3 - <<'PY'
from pathlib import Path
import re
text = Path("src/components/clinical-dashboard/master-search-header.tsx").read_text()
def extract_block(start_marker, end_marker):
start = text.index(start_marker)
end = text.index(end_marker, start + len(start_marker))
return text[start:end + len(end_marker)]
block = extract_block('const renderSearchComposer =', 'return null;')
print("== renderSearchComposer expression snippets ==")
for needle in ["className isHeroDesktopComposer", "isHeroDesktopComposer", "desktopHomeComposerHost ? createPortal", "desktopHomeComposerActive && desktopHomeComposerHost ? null", "desktopSearchPlacement!=", 'renderSearchComposer("default")']:
for m in re.finditer(re.escape(needle), block):
line = block[:m.start()].count("\n") + 1
start = max(0, m.start() - 220)
end = min(len(block), m.end() + 260)
ctx = block[start:end].splitlines()
print(f"\n[around line {line}] {needle}")
for raw in ctx:
print(raw)
model = [
{"desktopSearchPlacement": "hero", "desktopHomeComposerActive": False, "desktopHomeComposerHost": False},
{"desktopSearchPlacement": "hero", "desktopHomeComposerActive": True, "desktopHomeComposerHost": False},
{"desktopSearchPlacement": "hero", "desktopHomeComposerActive": True, "desktopHomeComposerHost": True},
]
print("\n== simple class resolution model ==")
for state in model:
is_hero = state["desktopSearchPlacement"] == "hero"
classes = "sm:hidden" if state["desktopHomeComposerActive"] and state["desktopHomeComposerHost"] and is_hero else ("sm:sticky sm:top-[calc(4.75rem+env(safe-area-inset-top))]")
show_default = state["desktopHomeComposerActive"] and state["desktopHomeComposerHost"]
print(state, "show_default_fallback", show_default, "classes=", classes)
PYRepository: BigSimmo/Database
Length of output: 304
Make the desktop fallback not invisible while the portal is attaching.
renderSearchComposer("default") keeps sm:hidden whenever isHeroDesktopComposer is true, so when desktopHomeComposerActive is true but desktopHomeComposerHost is still null, the default composer is mounted but hidden at desktop widths. Gate the hero class on portal attachment instead.
Proposed fix
- isHeroDesktopComposer
+ isHeroDesktopComposer && desktopHomeComposerActive && desktopHomeComposerHost
? "sm:hidden"
: "sm:sticky sm:top-[calc(4.75rem+env(safe-area-inset-top))]",Validate the desktop mode-home handoff with npm run ensure and npm run verify:ui.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/clinical-dashboard/master-search-header.tsx` around lines 1890
- 1895, Update renderSearchComposer and its desktop fallback condition so the
hero-specific sm:hidden class is applied only when the desktop home composer
portal is actually attached, not merely when isHeroDesktopComposer is true.
Ensure the default composer remains visible while desktopHomeComposerActive is
true and desktopHomeComposerHost is null, then validate with npm run ensure and
npm run verify:ui.
Source: Coding guidelines
| /** Drop a restored private-scope ref from the current URL without a navigation. */ | ||
| export function removePrivateScopeRefFromUrl(location: Pick<Location, "pathname" | "search"> = window.location) { | ||
| const params = new URLSearchParams(location.search); | ||
| params.delete("scopeRef"); | ||
| const next = params.toString(); | ||
| window.history.replaceState(null, "", `${location.pathname}${next ? `?${next}` : ""}`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the current hash fragment.
Clearing scopeRef also drops #sources (or another active section), breaking the current in-page location. Include hash in the location shape and append it to the replacement URL.
Proposed fix
-export function removePrivateScopeRefFromUrl(location: Pick<Location, "pathname" | "search"> = window.location) {
+export function removePrivateScopeRefFromUrl(
+ location: Pick<Location, "pathname" | "search" | "hash"> = window.location,
+) {
const params = new URLSearchParams(location.search);
params.delete("scopeRef");
const next = params.toString();
- window.history.replaceState(null, "", `${location.pathname}${next ? `?${next}` : ""}`);
+ window.history.replaceState(null, "", `${location.pathname}${next ? `?${next}` : ""}${location.hash}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Drop a restored private-scope ref from the current URL without a navigation. */ | |
| export function removePrivateScopeRefFromUrl(location: Pick<Location, "pathname" | "search"> = window.location) { | |
| const params = new URLSearchParams(location.search); | |
| params.delete("scopeRef"); | |
| const next = params.toString(); | |
| window.history.replaceState(null, "", `${location.pathname}${next ? `?${next}` : ""}`); | |
| /** Drop a restored private-scope ref from the current URL without a navigation. */ | |
| export function removePrivateScopeRefFromUrl( | |
| location: Pick<Location, "pathname" | "search" | "hash"> = window.location, | |
| ) { | |
| const params = new URLSearchParams(location.search); | |
| params.delete("scopeRef"); | |
| const next = params.toString(); | |
| window.history.replaceState(null, "", `${location.pathname}${next ? `?${next}` : ""}${location.hash}`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/private-search-scope.ts` around lines 75 - 80, Update
removePrivateScopeRefFromUrl to include hash in its Location shape and preserve
the current fragment when constructing the replacement URL. Append location.hash
after the pathname and updated query string so removing scopeRef does not change
the active in-page section.
|
Closing per open-PR hygiene review (2026-07-26). Prior review found a P1: isDashboardModeHref treats only /? as dashboard-resident, so Answer to Documents (/documents/search, which keeps ClinicalDashboard mounted) skips setSearchMode/setQuery and can fire an unintended answer generation under run=1. That guard is still present on current head 7bceebf. The branch is also heavily conflicting vs current main (globals.css, ClinicalDashboard, search shell/header, docs) and still carries a mismatched PR body. Do not merge. Re-implement on a fresh branch from main; gate leave-dashboard with shouldRenderClinicalDashboard so Documents stays on the in-shell path. |
Capture the 2026-07-26 close decisions and merge-ready keep list after re-triaging the Antigravity/Cursor queue. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
fields=index, differentials abort/debounce, universal documents typeahead soft-timeout (750ms), shared(search-app)shell to avoid composer remount, and Answer rate-limit in-memory fallback outside production./services→/dsm) syncssearchModeduring render (no stale-mode paint) even when the query string is unchanged; extracted ClinicalDashboard lazy imports to stay under the maintainability budget.RAG impact: no retrieval behaviour change — typeahead documents domain timeout and shell URL sync only; ranking formulas and full
/api/searchretrieval path unchanged.Verification
npm run verify:pr-local— focused Vitest on touched sources (362) plus api-rate-limit / search-shell / universal / route / site-map suites green;docs:check-indexOKverify:uinot required for this pass; mode-home smoke vianpm run ensurereturned HTTP 200 for/,/services,/dsm,/documents/search,/therapy-compass,/?mode=prescribing, and/api/answer/streamreturned 200 after the rate-limit fallback fixeval:retrieval:latency/ soak / live OpenAI canary — approval-gated provider work; not needed for timeout-only typeahead changeRisk and rollout
GlobalSearchShelllayouts and prior timeout/fallback behaviourClinical Governance Preflight
[REDACTED]([REDACTED])Notes
fields=indexremains for identity-only consumers (cross-mode links).Summary by CodeRabbit
New Features
Bug Fixes
Documentation